Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Geo Context Mapping Parsing More Strict (6.x version) #32862

Conversation

imotov
Copy link
Contributor

@imotov imotov commented Aug 14, 2018

Currently, if geo context is represented by something other than
geo_point or an object with lat and lon fields, the parsing of it
as a geo context can result in ignoring the context altogether,
returning confusing errors such as number_format_exception or trying
to parse the number specifying as long-encoded hash code. It would also
fail if the geo_point was stored.

This commit makes the deprecates creation of geo contexts without
correct path fields. And it fixes the issue when geo_point is stored.

This is a 6.x version of #32821.

Currently, if geo context is represented by something other than
geo_point or an object with lat and lon fields, the parsing of it
as a geo context can result in ignoring the context altogether,
returning confusing errors such as number_format_exception or trying
to parse the number specifying as long-encoded hash code. It would also
fail if the geo_point was stored.

This commit makes the deprecates creation of geo contexts without
correct path fields. And it fixes the issue when geo_point is stored.
It is a 6.x version of elastic#32821.
@imotov imotov added :Analytics/Geo Indexing, search aggregations of geo points and shapes :Search Relevance/Suggesters "Did you mean" and suggestions as you type >deprecation v6.5.0 labels Aug 14, 2018
@imotov imotov requested review from nknize and jimczi August 14, 2018 23:12
@imotov imotov changed the title Make Geo Context Mapping Parsing More Strict Make Geo Context Mapping Parsing More Strict (6.x version) Aug 14, 2018
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @imotov

Copy link
Contributor

@nknize nknize left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@imotov
Copy link
Contributor Author

imotov commented Aug 17, 2018

retest this please

@imotov imotov merged commit 724ee0c into elastic:6.x Aug 20, 2018
jasontedor pushed a commit that referenced this pull request Aug 21, 2018
Currently, if geo context is represented by something other than
geo_point or an object with lat and lon fields, the parsing of it
as a geo context can result in ignoring the context altogether,
returning confusing errors such as number_format_exception or trying
to parse the number specifying as long-encoded hash code. It would also
fail if the geo_point was stored.

This commit makes the deprecates creation of geo contexts without
correct path fields. And it fixes the issue when geo_point is stored.

This is a 6.x version of #32821.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Aug 24, 2018
* commit '9088d811ce9cff922e6ef1befbeb0f1e0c27016a': (23 commits)
  Generalize remote license checker (elastic#32971)
  Trim translog when safe commit advanced (elastic#32967)
  Fix an inaccuracy in the dynamic templates documentation. (elastic#32890)
  HLREST: AwaitsFix ML Test
  Make Geo Context Mapping Parsing More Strict (elastic#32862)
  Add mzn and dz to unsupported locales (elastic#32957)
  Use settings from the context in BootstrapChecks (elastic#32908)
  Update docs for node specifications (elastic#30468)
  HLRC: Forbid all Elasticsearch logging infra (elastic#32784)
  Fix use of deprecated apis
  Only configure publishing if it's applied externally (elastic#32351)
  Protect ScriptedMetricIT test cases against failures on 0-doc shards (elastic#32959) (elastic#32968)
  Scripted metric aggregations: add deprecation warning and system (elastic#32944)
  Watcher: Properly find next valid date in cron expressions (elastic#32734)
  Fix some small issues in the getting started docs (elastic#30346)
  Set forbidden APIs target compatibility to compiler java version   (elastic#32935)
  [TEST] Add "ne" as an unsupported SimpleKdc locale (elastic#32700)
  MINOR: Remove `IndexTemplateFilter` (elastic#32841) (elastic#32974)
  INGEST: Create Index Before Pipeline Execute (elastic#32786) (elastic#32975)
  NETWORKING: Make RemoteClusterConn. Lazy Resolve DNS (elastic#32764) (elastic#32976)
  ...
@colings86 colings86 removed the :Analytics/Geo Indexing, search aggregations of geo points and shapes label Nov 2, 2018
@imotov imotov deleted the issue-32202-incorrect-geo-context-path-mapping-time-6x branch May 1, 2020 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>deprecation :Search Relevance/Suggesters "Did you mean" and suggestions as you type v6.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants